[WC-3362] Charts: Add refresh interval to list-based charts - #2486

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml

⚠️ Note: gh pr checks was not available in this environment. CI status could not be verified automatically — please confirm all checks are green before merging.

Findings

⚠️ Low — Missing test for negative intervalSeconds

File: packages/shared/charts/src/hooks/__tests__/useRefreshInterval.spec.ts
Note: The spec (spec.md) states "When refreshInterval is 0 or negative, the chart SHALL NOT schedule any automatic reload." The implementation correctly uses intervalSeconds > 0, but there is no test covering a negative value (e.g. -1). Existing tests only cover the 0 case. Adding one is a one-liner and makes the spec compliance explicit.

it("does not reload when interval is negative", () => {
    const ds = new ListValueBuilder().build();
    setup({ datasources: [ds], interval: -1 });
    jest.advanceTimersByTime(60_000);
    expect(ds.reload).not.toHaveBeenCalled();
});

⚠️ Low — No automated E2E spec for the new interactive feature

File: packages/pluggableWidgets/line-chart-web/e2e/ (does not exist for this feature)
Note: Task 5.2 is explicitly left unchecked ("Manual QA in Studio Pro"). The refresh interval is timer-driven and user-visible, which makes it a good candidate for an E2E spec. Not blocking — the unit tests cover all spec-required behaviors comprehensively — but an E2E spec matching the PR's manual test plan would guard against regressions. Consider adding one before the next release.

Positives

• useRefreshInterval.spec.ts covers all 9 spec-required behaviors (disabled at 0, single fire, multi-source, timer restart on data change, same-datasource no restart, loading-source skip, interval change, StrictMode remount, unmount cancellation). The StrictMode double-invoke case in particular shows careful React 18 awareness.
• The ref-based architecture (latestRef for the callback, prevDatasourcesRef for change detection) cleanly avoids stale closures without spreading a variable-length array into a useEffect dependency list — the design doc explains the rationale clearly.
• seriesDataSources is extracted as a separately-tested pure helper, making the per-widget call sites read as a one-liner and keeping the hook API generic.
• All custom memo comparators (containerPropsEqual, LineChart/TimeSeries/BubbleChart custom flatEqual comparators) use defaultEqual (Object.is) for non-series props, so refreshInterval changes correctly trigger re-renders without any comparator modification.
• Default refreshInterval = 0 in all XMLs makes the change fully backward-compatible with no migration path needed.
• XML property key refreshInterval is lowerCamelCase and matches the TypeScript interface exactly across all 8 widgets.